Skip to content

refactor: centralize XML import payload loading#280

Merged
TheWitness merged 1 commit intoCacti:developfrom
somethingwithproof:refactor/shared-import-payload-loader
Mar 16, 2026
Merged

refactor: centralize XML import payload loading#280
TheWitness merged 1 commit intoCacti:developfrom
somethingwithproof:refactor/shared-import-payload-loader

Conversation

@somethingwithproof
Copy link
Contributor

Summary

Fixes #277

Validation

  • php -l functions.php
  • php -l syslog_alerts.php
  • php -l syslog_reports.php
  • php -l syslog_removal.php
  • php -l tests/regression/issue277_import_payload_loader_test.php
  • php tests/regression/issue277_import_payload_loader_test.php

Copilot AI review requested due to automatic review settings March 7, 2026 03:03
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR centralizes the duplicated “XML import payload loading” logic (text area vs uploaded file, otherwise redirect/exit) into a shared helper in functions.php, and updates the alert/report/removal import handlers to use it—addressing issue #277.

Changes:

  • Add syslog_get_import_xml_payload($redirect_url) helper to load XML from request text or uploaded file, otherwise redirect/exit.
  • Update alert_import(), report_import(), and removal_import() to use the shared helper.
  • Add a regression test ensuring all three handlers call the helper and remove legacy per-file upload handling; update changelog.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
functions.php Introduces shared XML import payload loader helper.
syslog_alerts.php Replaces duplicated payload loading with helper call in alert_import().
syslog_reports.php Replaces duplicated payload loading with helper call in report_import().
syslog_removal.php Replaces duplicated payload loading with helper call in removal_import().
tests/regression/issue277_import_payload_loader_test.php Adds regression test to enforce helper usage and legacy logic removal.
CHANGELOG.md Documents issue #277 change.

@somethingwithproof somethingwithproof force-pushed the refactor/shared-import-payload-loader branch from 052bf88 to 1882acb Compare March 7, 2026 13:37
Copy link
Member

@TheWitness TheWitness left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another good one. Fix the merge conflicts and we should be okay.

@somethingwithproof somethingwithproof force-pushed the refactor/shared-import-payload-loader branch from 45ff495 to 6d038f3 Compare March 10, 2026 20:30
@somethingwithproof
Copy link
Contributor Author

Rebased, CHANGELOG conflicts resolved.

somethingwithproof added a commit to somethingwithproof/plugin_syslog that referenced this pull request Mar 10, 2026
Refs Cacti#280

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
@somethingwithproof somethingwithproof force-pushed the refactor/shared-import-payload-loader branch from 6d038f3 to b63a05c Compare March 10, 2026 20:55
somethingwithproof added a commit to somethingwithproof/plugin_syslog that referenced this pull request Mar 12, 2026
Refs Cacti#280

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
@somethingwithproof somethingwithproof force-pushed the refactor/shared-import-payload-loader branch from b63a05c to 84ac26e Compare March 12, 2026 00:18
Refs Cacti#280

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
@somethingwithproof somethingwithproof force-pushed the refactor/shared-import-payload-loader branch from 84ac26e to 317c3c0 Compare March 15, 2026 05:17
@TheWitness TheWitness merged commit 4aceb63 into Cacti:develop Mar 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor: centralize XML import payload loading logic for alert/report/removal rules

3 participants